Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

service discovery: add initial MVP implementation #12368

Merged
merged 45 commits into from
Mar 25, 2022
Merged

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Mar 24, 2022

This PR contains the MVP work for Nomad service discovery, whereby Nomad services can specify a provider parameter and become registered within Nomad state. It includes state, RPC, HTTP API, CLI and client work. It does not currently include template functionality which is being worked on currently.

All commits have been reviewed previously except the final two. These two commits perform small refactoring to accommodate CI changes on main and a naming change discussed internally.

I was planning on merging this without squashing to keep some history of the development rather than a single commit which contains +10k changes. If there are thoughts on why we should squash then I am happy to alter the approach.

The failing Nomad RPC test is not a result of changes here, and is fixed within #12364

jrasell and others added 30 commits February 28, 2022 10:14
service discovery: add state store functionality
events: add state objects and logic for service registrations.
service discovery: add RPC endpoints and FSM logic
service discovery: add HTTP endpoints and sdk wrapper
jobspec: add service block provider parameter
config: add native service discovery admin boolean parameter.
service discovery: add config boolean parameter and fingerprinting
This commit performs refactoring to pull out common service
registration objects into a new `client/serviceregistration`
package. This new package will form the base point for all
client specific service registration functionality.

The Consul specific implementation is not moved as it also
includes non-service registration implementations; this reduces
the blast radius of the changes as well.
client: refactor common service registration objects from Consul.
jrasell and others added 11 commits March 17, 2022 09:17
client: add Nomad service registration implementation.
client: hookup service wrapper for use in clients
When a node fails its heart beating a number of actions are taken
to ensure state is cleaned. Service registrations a loosely tied
to nodes, therefore we should remove these from state when a node
is considered terminally down.
cli: add service commands for list, info, and delete.
The service registration client name was used to provide a
distinction between the service block and the service client. This
however creates new wording to understand and does not match the
CLI, therefore this change fixes that so we have a Services
client.

Consul specific objects within the service file have been moved to
the consul location to create a clearer separation.
@jrasell jrasell added this to the 1.3.0 milestone Mar 24, 2022
@jrasell jrasell requested a review from schmichael March 24, 2022 10:27
@jrasell jrasell self-assigned this Mar 24, 2022
Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! (last two commits)

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants